Codecov Report

❌ Patch coverage is 87.05674% with 73 lines in your changes missing coverage. Please review.

📢 Thoughts on this report? Let us know!

@xgopilot 先确认当前总体方案设计，再确认有没有存在风险点（诸如方案设计、模块边界等等）

审查结论

基于 origin/main...HEAD 对当前 PR 做了方案与边界审查，发现以下风险点（按严重级别排序）：

1. High: runtime.assets 限额在 compact 分支未统一走 ResolveSelectedProvider，链路存在不一致

   • 位置：internal/runtime/compact.go:179
   • 问题：当会话里已有 session.Provider/session.Model 时，走 providerCfg.Resolve()，不会注入 cfg.Runtime.ResolveSessionAssetLimits()；而主运行链路走 ResolveSelectedProvider(cfg)。这与本 PR“限额全链路一致”的目标不一致。
   • 风险：不同执行路径的 provider.RuntimeConfig.SessionAssetLimits 可能不一致，后续若 compact 或其衍生请求带附件，将出现行为漂移。
   • 建议：compact 分支统一改为调用 config.ResolveSelectedProvider 风格的注入逻辑，或在 ProviderConfig.Resolve() 内集中注入。

2. Medium: anthropic driver 现在在 Build 阶段无条件失败，且已去掉可绕行配置面

   • 位置：internal/provider/anthropic/driver.go:21-25
   • 问题：Build 直接返回 not supported；同时本 PR 已移除原先可配置的协议字段（chat_protocol/api_style 等），意味着无法通过配置切到可执行聊天协议。
   • 风险：driver=anthropic 在运行时成为“可配置但不可用”，容易造成用户侧误配置与排障成本上升。
   • 建议：若当前仅支持 discovery，应在配置/TUI层显式标注“discovery-only”或临时隐藏该 driver 的可选入口。

3. Medium: 自定义 provider 配置迁移是“硬失败”策略，缺少渐进迁移兜底

   • 位置：internal/config/provider_loader.go:96-100（KnownFields(true)）
   • 问题：旧版嵌套字段（openai_compatible/gemini/anthropic）会直接解析失败。
   • 风险：升级后若用户目录下存在旧配置，启动阶段可能整体失败，影响可用性。
   • 建议：至少增加一次性迁移提示（指出具体文件路径与替换字段），或提供自动迁移命令/脚本。

模块边界检查

• 本次收敛方向基本符合“协议差异下沉到 provider 层、配置/TUI 简化”的目标。
• 当前主要边界风险集中在“同一能力在不同 runtime 分支上的注入一致性”与“UI 暴露能力与 driver 实际可执行能力不一致”。

本轮执行范围

• 已完成：方案设计与风险点审查。
• 未执行：代码修改、提交与推送（本次触发为分析请求）。

View job run

@xgopilot 给我修复第一个和第三个问题，第三个问题你只能以脚本/命令方式解决

处理结果

已按你的要求只修复第 1、3 个问题，并已推送到当前 PR 分支。

变更说明

1. 问题 1（compact 分支 assets 注入不一致）

• 已修复 internal/runtime/compact.go：resolveCompactProviderSelection 在“复用会话 provider/model”分支下，统一注入 cfg.Runtime.ResolveSessionAssetLimits()。
• 新增回归测试 internal/runtime/runtime_remaining_branches_test.go：TestResolveCompactProviderSelectionSessionBranchInjectsRuntimeAssetLimits。

2. 问题 3（旧 provider.yaml 迁移兜底，按脚本/命令方式）

• 新增迁移脚本：scripts/migrate_provider_yaml.go
• 新增脚本测试：scripts/migrate_provider_yaml_test.go
• 脚本能力：
  • 扫描并迁移 ~/.neocode/providers/*/provider.yaml（或通过 --target 指定文件/目录）
  • 将旧嵌套块 openai_compatible/gemini/anthropic 收敛为扁平字段 base_url/chat_endpoint_path/discovery_endpoint_path
  • 迁移时自动生成 .bak 备份
  • 支持 --dry-run 预览
• 同步更新文档：docs/guides/configuration.md（改为新扁平示例并加入迁移命令）

使用命令

go run ./scripts/migrate_provider_yaml.go --base-dir ~/.neocode

仅预览：

go run ./scripts/migrate_provider_yaml.go --base-dir ~/.neocode --dry-run

验证

• go test ./internal/runtime
• go test ./scripts
• go test ./...
• go build ./...

提交与推送

• Commit: 7171314
• 已推送分支：phantom5099:fork-pr-367-1776585038
• Fork PR（同步记录）：fix: unify compact asset injection and add provider yaml migration script phantom5099/neo-code#42

未处理项：第 2 个问题（按你的指示本次不处理）。

View job run

@xgopilot 给我一个能给AI用来review的提示词，先确认当前总体方案设计，再确认有没有存在风险点（诸如方案设计、模块边界等等）

@xgopilot 你是资深 Go 架构评审。请只做评审，不做代码修改。

评审对象：当前 PR（base=origin/main, compare=origin/main...HEAD）
仓库目标主链路：用户输入 -> Agent 推理 -> 调用工具 -> 获取结果 -> 继续推理 -> UI 展示

请按以下顺序输出：

1. 总体方案确认（先做）

• 用 5~10 条要点确认这次 PR 的“设计意图是否自洽”。
• 重点核对：
  • 是否符合分层边界：TUI -> Runtime -> Provider/ToolManager
  • provider 差异是否收敛在 internal/provider，是否泄漏到 runtime/tui/config
  • runtime 是否只做编排，不承接 provider 协议细节
  • tools 能力是否仍统一经 internal/tools 暴露
  • session/runtime/config 对状态和限额（如 asset limits）是否全链路一致
• 若发现“目标与实现不一致”，直接标注为风险候选。

2. 风险点审查（再做）

• 仅输出“问题/风险”，不要写表扬。
• 按严重级别排序：High -> Medium -> Low。
• 每条风险必须包含：
  • 标题（一句话）
  • 证据（文件路径+行号+关键行为）
  • 为什么是风险（可能导致的行为偏差/边界破坏/回归）
  • 建议修复方向（最小改动优先）
  • 建议补测点（正常/边界/异常/回归）

3. 必查清单（逐项明确“通过/不通过/部分通过”）

• 配置模型收敛后是否保持严格校验且可迁移
• 运行时分支是否存在配置注入不一致
• 会话存储、provider 请求构建、runtime 限额是否一致
• UI 暴露能力与底层 driver 实际能力是否一致
• 错误包装与事件派发是否可观测、可定位

4. 输出格式要求

• 使用以下结构：
  • 审查结论（1段）
  • 发现的问题（编号列表，按严重级别）
  • 开放问题/假设（如有）
  • 建议优先级修复顺序（1,2,3）
• 如果“无问题”，明确写：
  • “未发现阻断性问题”，并列出残余风险与测试盲区。

附加约束：

• 只基于代码与变更事实，不猜测作者意图。
• 不要跨层建议（例如让 tui 直接处理 provider 协议）。
• 不要给大重构方案，优先可落地的最小修复建议。

审查结论

本次 PR 在“协议差异收敛到 provider 层、runtime 只做编排、TUI 不直连协议细节、asset limits 贯通链路”这几个主目标上基本成立；但存在 1 个升级阻断风险、2 个行为一致性风险和 1 个文档失真风险，建议在合并前至少处理前 3 项。

总体方案确认（设计意图自洽性）

• TUI -> Runtime -> Provider 主链路保持成立：TUI 通过 CreateCustomProvider/Submit，未见跨层执行 provider 协议逻辑。
• provider 差异收敛方向成立：driver 默认协议/鉴权/discovery 通过 internal/provider/driver_defaults.go 统一解析。
• runtime 编排边界基本成立：附件限额注入在输入准备与 provider 请求构建环节，runtime 本身未下沉协议细节。
• tools 统一入口未被破坏：本 PR 未引入 runtime/tui 直接工具执行分支。
• session/runtime/provider 的附件限额链路基本打通：runtime -> session store -> openaicompat request 均接入 SessionAssetLimits。
• 风险候选：旧版 provider.yaml 在严格解析下会直接失败，迁移能力存在但错误引导不足。
• 风险候选：配置到 runtime 的协议归一化错误在 ToRuntimeConfig 被静默吞掉，不符合 fail-fast 目标。
• 风险候选：runtime.assets 的 0 语义与“限额可控”存在认知偏差（会被回填为默认 20MiB）。

发现的问题

1. [High] 旧版 provider.yaml 升级路径会直接阻断启动，且错误提示缺少迁移指引
   证据：internal/config/provider_loader.go:96-100 使用 decoder.KnownFields(true) 且直接返回解析错误；旧嵌套字段会被拒绝。
   为什么是风险：已有用户升级后若未先手动迁移，配置加载会 hard-fail，主链路不可用。当前报错对最终用户不够可操作。
   建议修复方向：在解析错误分支识别旧字段特征（如 openai_compatible/gemini/anthropic），包装为可执行迁移提示（包含 go run ./scripts/migrate_provider_yaml.go --base-dir ...）。
   建议补测点：

• 正常：新扁平字段可加载。
• 边界：旧字段存在但可迁移。
• 异常：旧字段 + 非法值时提示仍包含迁移指引。
• 回归：迁移后再次加载成功。

2. [Medium] ToRuntimeConfig 吞掉协议归一化错误，存在静默降级分支
   证据：internal/config/provider.go:210：normalizedProtocols, _ := normalizeProviderProtocolSettingsFromConfig(...) 忽略 err。
   为什么是风险：一旦 ResolvedProviderConfig 来源不是“已校验快照”（例如后续调用方直接构造或二次修改），会在运行期悄悄落到空路径/默认分支，偏离 fail-fast。
   建议修复方向：最小改动是让 ToRuntimeConfig 返回 (provider.RuntimeConfig, error) 并在调用链显式处理；至少不能静默忽略。
   建议补测点：

• 正常：合法配置转换成功。
• 边界：chat/discovery path 非法时返回错误。
• 异常：调用方收到错误后不进入 provider 构建。
• 回归：现有合法路径行为不变。

3. [Medium] runtime.assets 的 0 值会被回填默认值，和“限额可控”语义可能冲突
   证据：internal/config/runtime.go:96-101（<=0 回填默认）；internal/config/runtime.go:116-120 + internal/provider/types/asset_limits.go:27-32（再次归一化为默认 20MiB）。
   为什么是风险：运维将 0 作为“禁用附件”时，实际会放开到默认上限，可能产生策略偏差。
   建议修复方向：二选一并文档化：

• 将 0 设为非法（Validate 报错），或
• 将 0 解释为严格 0 并在 session/provider 双端执行拒绝。
  建议补测点：
• 正常：正数限额按配置生效。
• 边界：0 的预期语义（报错或严格禁用）一致。
• 异常：负数仍拒绝。
• 回归：默认值路径保持现状。

4. [Low] 配置文档未完整反映 runtime 新字段，存在实现-文档失真
   证据：docs/guides/configuration.md:88-93 仅描述 runtime.max_no_progress_streak；实现中已有 MaxRepeatCycleStreak 与 runtime.assets.*（internal/config/runtime.go:16-25）。
   为什么是风险：用户无法根据文档正确配置并验证附件限额与重复循环阈值。
   建议修复方向：补齐 runtime.max_repeat_cycle_streak、runtime.assets.max_session_asset_bytes、runtime.assets.max_session_assets_total_bytes 的说明与示例。
   建议补测点：文档类无单测；建议在配置加载测试中增加示例 YAML 回归用例。

必查清单

• 配置模型收敛后是否保持严格校验且可迁移：部分通过（严格校验成立；迁移可用但失败提示不够可操作）。
• 运行时分支是否存在配置注入不一致：部分通过（主链路一致；ToRuntimeConfig 存在静默错误吞并）。
• 会话存储、provider 请求构建、runtime 限额是否一致：部分通过（链路已贯通；0 语义与预期策略存在偏差风险）。
• UI 暴露能力与底层 driver 实际能力是否一致：通过（UI 字段收敛后与 driver 默认策略一致）。
• 错误包装与事件派发是否可观测、可定位：部分通过（runtime 事件链路可观测；provider 迁移失败场景定位信息不足）。

开放问题/假设

• runtime.assets.* = 0 的产品语义是否定义为“禁用附件”还是“使用默认值”？当前实现为后者，建议明确。

建议优先级修复顺序

1. 先修复旧版 provider.yaml 失败时的迁移引导（阻断升级）。
2. 修复 ToRuntimeConfig 错误吞并，恢复 fail-fast。
3. 明确并固化 runtime.assets 的 0 语义（实现+测试+文档同步）。

View job run

@xgopilot 修复第二个、第三个、第四个问题

已按你的要求修复第 2、3、4 项，并已提交推送到当前 PR 分支 phantom5099:main。

变更摘要

• 问题 2（ToRuntimeConfig 吞错）

  • internal/config/provider.go
  • 将 ResolvedProviderConfig.ToRuntimeConfig 从返回单值改为返回 (provider.RuntimeConfig, error)，不再忽略协议归一化错误。
  • 调用链全部改为显式错误处理：
    • internal/runtime/run.go
    • internal/runtime/compact.go
    • internal/runtime/subagent_engine.go
    • internal/app/bootstrap.go
    • internal/config/state/model.go
    • internal/provider/catalog/service_test.go

• 问题 3（runtime.assets=0 语义）

  • 语义已明确并固化为：0 或未配置 => 回退默认限制（20 MiB）。
  • 补充测试覆盖该行为：
    • internal/config/runtime_test.go
    • internal/provider/types/asset_limits_test.go
  • 相关注释已同步更新：
    • internal/config/runtime.go

• 问题 4（文档失真）

  • docs/guides/configuration.md 已补齐：
    • runtime.max_repeat_cycle_streak
    • runtime.assets.max_session_asset_bytes
    • runtime.assets.max_session_assets_total_bytes
    • 并在文档中明确 0 的行为语义。

测试

• 已执行：go test ./...
• 结果：全部通过。

提交信息

• Commit: 06bc56f
• 已推送：fork/main（对应当前 PR head 分支）

View job run

/code 提高测试覆盖率

处理结果

已完成“提高测试覆盖率”的补测，并推送到当前 PR 分支。

本次补测

• 新增 internal/provider/driver_defaults_test.go：覆盖 ResolveDriverProtocolDefaults、ResolveDriverDiscoveryConfig、ResolveDriverAuthConfig 的 driver 分支与异常路径。
• 扩展 scripts/migrate_provider_yaml_test.go：覆盖 runMigration、collectProviderFiles、collectProviderFilesFromProvidersDir、migrateProviderFile、selectLegacyBlock、defaultBaseDir 的关键边界和错误分支（含 dry-run/target 校验/invalid yaml）。
• 扩展 internal/provider/chat_endpoint_test.go：补齐非 / 前缀路径拼接与非法绝对 endpoint 路径错误分支。
• 扩展 internal/provider/types/asset_limits_test.go：补齐 DefaultSessionAssetLimits。

验证

• go test ./internal/provider ./internal/provider/types ./scripts
• go test ./... -coverprofile=/tmp/cover2.out
• 覆盖结果（关键文件）：
  • internal/provider/driver_defaults.go: 100%
  • internal/provider/chat_endpoint.go: 100%（ResolveChatEndpointURL）
  • internal/provider/types/asset_limits.go: 100%
  • scripts/migrate_provider_yaml.go: 71.9%（由 45.6% 显著提升）

提交与推送

• Commit: f009812
• 已推送分支：phantom5099/fork-pr-367-1776585038
• Fork PR: test: improve coverage for provider defaults and migration script phantom5099/neo-code#43

未执行项：未修改功能逻辑，仅做测试覆盖增强。

View job run

/review

/code 修复最近两条review的问题

修复结果

已按“最近两条 review”完成修复并推送到当前 PR 分支。

1. 修复 SaveCustomProviderWithModels 读写契约不一致

• 文件：internal/config/provider_loader.go
• 调整：在 model_source=manual 分支接入 validateModelDescriptorsRequireName，写入前强制校验 models[].id/name。
• 结果：不再出现“保存成功、下次加载因 models[].name 为空而失败”的情况。

2. 更新文档去除过期字段

• 文件：docs/guides/configuration.md
• 调整：删除 manual 模式说明中已移除字段 discovery_response_profile 的描述，仅保留 discovery_endpoint_path。

3. 同步测试

• 文件：internal/config/loader_test.go
• 调整：TestSaveCustomProviderRejectsModelWithoutName 改为断言保存时报错（models[0].name is empty）。

验证

• go test ./internal/config ./internal/config/state 通过。

提交与推送

• Commit: e3d8825
• 已推送：phantom5099/fork-pr-367-1776585038
• Fork PR（同步记录）：fix(config): address latest review findings phantom5099/neo-code#44

未执行项：无。

View job run